Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(appsec): add auto_user_instrum remote config capabilities #3079

Merged
merged 8 commits into from
Feb 21, 2025
Merged

feat(appsec): add auto_user_instrum remote config capabilities #3079

merged 8 commits into from
Feb 21, 2025

Conversation

Leiyks
Copy link
Contributor

@Leiyks Leiyks commented Feb 11, 2025 •
edited
Loading

Description

Enable remote configs related to the auto_user_instrumentation mode according to the following RFC.

The following changes have been implemented:

  • Helper: Add a new config aggregator for ASM_FEATURES to allow single file parsing in the listener.
  • Helper/Extension: Add the new settings field in the requests_(init | exec | shutdown) responses to communicate parameters.
  • Extension: Parsing methods for the additional settings field and new mechanism to get the auto_user_instrumentation mode.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

Related Jiras: APPSEC-53750

@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 73.46939% with 13 lines in your changes missing coverage. Please review.

Project coverage is 73.06%. Comparing base (368a541) to head (864cab2).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
appsec/src/extension/commands_helpers.c 66.66% 5 Missing and 3 partials ⚠️
appsec/src/extension/user_tracking.c 80.00% 2 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3079      +/-   ##
============================================
+ Coverage     73.04%   73.06%   +0.02%     
  Complexity     2792     2792              
============================================
  Files           139      139              
  Lines         15284    15330      +46     
  Branches       1047     1063      +16     
============================================
+ Hits          11164    11201      +37     
- Misses         3567     3572       +5     
- Partials        553      557       +4
Flag Coverage Δ
appsec-extension 68.46% <73.46%> (+0.17%) ⬆️
tracer-php 74.85% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
appsec/src/extension/user_tracking.c 73.43% <80.00%> (+2.68%) ⬆️
appsec/src/extension/commands_helpers.c 63.72% <66.66%> (+1.15%) ⬆️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 368a541...864cab2. Read the comment docs.

@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch from ddeb6ec to 271f4f0 Compare February 11, 2025 09:46
@pr-commenter
Copy link

pr-commenter bot commented Feb 11, 2025 •
edited
Loading

Benchmarks [ appsec ]

Benchmark execution time: 2025-02-21 14:15:57

Comparing candidate commit 864cab2 in PR branch leiyks-rc-user-instrum-mode with baseline commit 368a541 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.

@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch 4 times, most recently from e1ff2b3 to b989ad6 Compare February 11, 2025 16:19
@Leiyks Leiyks marked this pull request as ready for review February 11, 2025 16:27
@Leiyks Leiyks requested a review from a team as a code owner February 11, 2025 16:27
@Leiyks Leiyks requested a review from estringana February 12, 2025 14:04
@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch 2 times, most recently from 074bca8 to 351931d Compare February 12, 2025 15:40
Anilm3
Anilm3 requested changes Feb 18, 2025
View reviewed changes
Copy link
Contributor

@Anilm3 Anilm3 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments, the only critical issue is on the listener.

@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch 7 times, most recently from e21821e to 69e8880 Compare February 19, 2025 15:00
@Leiyks Leiyks requested a review from Anilm3 February 19, 2025 16:11
@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch from 69e8880 to 1ff318a Compare February 20, 2025 10:05
@Leiyks Leiyks requested a review from Anilm3 February 20, 2025 10:05
@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch from 1ff318a to 087b081 Compare February 20, 2025 12:21
@Leiyks Leiyks requested a review from Anilm3 February 20, 2025 12:26
@Leiyks
feat(helper: remote_config): add config aggregator for asm features
f81fabf 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks
feat(helper: client): add settings field to requests
69313e9 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks
feat(extension: user_tracking): add quto_user_instrum capability
750a8c6 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks
test(integrations): add auto_user_instrum rc tests
e3bd08b 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks
chore: restore end-of-line newline
3bbe4aa 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks
chore: apply feedbacks
0086536 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks
feat(extension): add warning logs on unexpected cases
c8d4485 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch 2 times, most recently from e31b4f7 to f41b9fc Compare February 21, 2025 13:32
Anilm3
Anilm3 approved these changes Feb 21, 2025
View reviewed changes
Copy link
Contributor

@Anilm3 Anilm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Leiyks
chore: apply feedbacks
864cab2 
Signed-off-by: Alexandre Rulleau <[email protected]>
@Leiyks Leiyks force-pushed the leiyks-rc-user-instrum-mode branch from f41b9fc to 864cab2 Compare February 21, 2025 13:39
@Leiyks Leiyks merged commit 9f5ae0a into master Feb 21, 2025
739 of 758 checks passed
@Leiyks Leiyks deleted the leiyks-rc-user-instrum-mode branch February 21, 2025 14:58
@github-actions github-actions bot added this to the 1.8.0 milestone Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Anilm3 Anilm3 Anilm3 approved these changes

@estringana estringana Awaiting requested review from estringana

Assignees
No one assigned
Labels
area:asm tracing
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.
4 participants
@Leiyks @codecov-commenter @Anilm3 @estringana